Conversation
|
Wow, definitely thorough :) So do you think this should just be put alongside the existing scanner package, or should it supersede it? |
tokenizer/token.go
Outdated
| } | ||
|
|
||
| // Attempt to turn the token back into a CSS string. | ||
| func (t *Token) WriteTo(w io.Writer) { |
There was a problem hiding this comment.
This is causing go vet to raise an error in the Travis tests. The signature should be func (t *Token) WriteTo(w io.Writer) (int64, error) with the same kind of semantics as the interface (return the number of bytes written and any error).
There was a problem hiding this comment.
Yeah, I figured that was going to happen. Do you think I should just do Render() string or implement the byte counting?
There was a problem hiding this comment.
At the least it needs to return the error from the underlying writer, so you might as well include the byte counting.
There was a problem hiding this comment.
It should be pretty easy if you eliminate the uses of fmt and just use io.WriteString everywhere
There was a problem hiding this comment.
Ended up using a helper function, called it stickyWriteString(n *int64, err *error, w io.Writer, s string) after the concept of "sticky errors". Not the best name but /shrug
Also updated the README and did another pass over documentation.
riking
changed the title
|
|
||
| // Handle first character | ||
| // dashes allowed at start only for TokenIdent-ish | ||
| // eE not allowed at start for Dimension |
There was a problem hiding this comment.
What was the reasoning behind this? It causes something like asd {sdf: 3em} to incorrectly turn into asd {sdf: 3\65 m} when re-encoding.
There was a problem hiding this comment.
I think this was referenced somewhere as being due to 1e9px or something like that. Check the spec?
|
@riking This pull request has some complex conflicts that need to be resolved and then we can review it. Thanks! |
5 participants
I needed a tokenizer that properly skipped bad-string and bad-url without erroring out, so I just rewrote the whole thing /shrug
Expanded the tests and includes fuzz testing.